build: remove setup.py and other build changes#79
Conversation
jandom
added
the
safe-to-test
jandom
added
safe-to-test
jandom
added
safe-to-test
jandom
added
safe-to-test
jnwei
left a comment
jnwei
left a comment
There was a problem hiding this comment.
Overall this looks great! Thank you for cleaning up the environment handling for the docker image.
IMO, I think if we're not fully supporting osx envrionments, we shouldn't include the experimental builds. Instead, I would suggest we recommend folks to use the OpenFold3-MLX project for support on Mac hardware.
| - awscli | ||
| - setuptools | ||
| - pip | ||
| - conda-forge::uv |
There was a problem hiding this comment.
nit: shouldn't uv be installed from conda-forge by default?
There was a problem hiding this comment.
Thank you for the review!
The OpenFold3-MLX is a slightly different story, since they want to re-write everything in this MLX framework. I'd like to support people running the CPU-native workloads (datapipeline) of OpenFold on Macs. But it's a bit experimental so I'd agree not to signal that we can support it (we can't yet, it'd be a lot of work).
nit: shouldn't uv be installed from conda-forge by default?
it should, i guess this is marginally more explicit (if another random channel provides this package, ignore it) but to be fair I don't really remember how the package vs multiple-channels thing work.
There was a problem hiding this comment.
Actually, let's maybe keep the osx and say it's 'experimental' – maybe some members of the community will contribute off a nugget like this. People seem to be quite interested in building OF3 for their various platforms (DGX etc). What do you think?
There was a problem hiding this comment.
I haven't tested running OpenFold on CPUs, but my understanding is that the capacity of structures that could be predicted is quite limited because the triangle attention modules are optimized for GPUs. Did you already have a chance to test the osx builds on your machine?
If we haven't tested the osx build and/or it doesn't work on very many use cases, I would suggest omitting the osx builds for now. I think that organizing our environment files with production-linux-64.* provides a reference format for future contributors to add environments for other platforms that they've tested.
There was a problem hiding this comment.
No, no really tested - okay, you've convinced me, I've removed the OSX stuff completely. I think this is ready to merge :D
jandom
added
safe-to-test
* build: remove setup.py * factor out test deps into pyproject.toml * time to add uv to support dep groups * playing with osx-arm64 support * typo: missed an extension * review: comments from Jennifer
…fixes Fix checkpoint loading for finetuning from pretrained checkpoint
2 participants
Summary
This PR provides changes to gradually harmonize some of the build process. Less moving pieces, and closer to best practices.
Changes
pip install --no-deps .)requirements-test.txtintopyproject.toml, using pip "dependency group" to bake this intopyproject.tomland install only the test deps without the main deps.pre-commit-config.yamlbecause it's personal for nowRelated Issues
The 'main' deps declared in
pyproject.tomlwill break the production environment. I've confirmed that setup.py route only worked because it skipped all the deps declared in pyproject.toml :)Testing
Locally all dandy, launching a test job now.
Other Notes